refactor(s2): validate CRS/EPSG and bbox attrs via typed constructors (#180 parity) - #217
Draft
lhoupert wants to merge 1 commit into
Draft
refactor(s2): validate CRS/EPSG and bbox attrs via typed constructors (#180 parity)#217lhoupert wants to merge 1 commit into
lhoupert wants to merge 1 commit into
Conversation
…#180 parity) Extend the typed-CRS/bbox idiom from the S1 RTC review (9ebad3e) to the pre-existing S2/generic conversion code: - types.py: add EPSGCode NewType + make_epsg_code validating constructor, the integer-code sibling of CRSCode/make_crs_code. Accepts 32631, "32631" and "EPSG:32631" (the forms found in stored attrs and CPM 2.6.0 metadata); corrupt values fail loudly with a clear TypeError. - s2_converter.py: route the CPM-2.6.0 hand-parsing and the proj:epsg read through make_epsg_code; _as_bbox delegates to make_bounding_box (keeping its lenient skip-on-malformed walker semantics); the store-root bbox walker validates proj:code via make_crs_code and carries BoundingBox2D. - conversion/geozarr.py, s2_multiscale.py: proj:epsg / CPM-2.6.0 reads via make_epsg_code. - s1_ingest.py: BACKSCATTER_CF_ATTRS gains the Final annotation its sibling TIME_CF_ATTRS already has. - tests: make_epsg_code unit tests; test_write_geo_metadata_invalid_crs now expects the earlier, typed error (TypeError vs pyproj CRSError deeper in the stack) — the one intentional behavior change. No on-disk output changes. Full-project pyright: 0 errors; pre-commit green; full non-network suite passes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #180: extends the typed-CRS/bbox refactor (9ebad3e) to the S2/generic conversion code. Stacked on #216 — retarget to
mainonce it merges.Summary
eopf_geozarr.types: addEPSGCodeNewType +make_epsg_codevalidating constructor (integer sibling ofCRSCode/make_crs_code; accepts32631,"32631","EPSG:32631"), with unit tests.proj:epsg/ CPM-2.6.0 CRS reads ins2_converter.py,s2_multiscale.pyandconversion/geozarr.pythrough it;_as_bboxnow delegates tomake_bounding_box; the store-root bbox walker validatesproj:codeand carriesBoundingBox2D.BACKSCATTER_CF_ATTRSgains its missingFinalannotation.For reviewers
Typing/validation only — no conversion-logic or on-disk changes. The one intentional behavior change: corrupt CRS attrs now fail early with a clear
TypeErrorinstead of pyproj'sCRSErrordeeper in the stack (see updatedtest_write_geo_metadata_invalid_crs). Verified: pyright 0 errors, pre-commit green, full non-network suite passes.Author attestation
every change and can explain why each is correct.
Implemented with AI assistance (Claude Code); reviewed by me and verified via pyright, pre-commit and the full non-network test suite.